-
Notifications
You must be signed in to change notification settings - Fork 18
Add some marine variables #131
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Tagging @travissluka, @shlyaeva, @Dooruk |
|
There's a failing check on this PR that should be resolved by commit 2cfaeb5, but the test didn't get re-triggered. |
|
Note that I'm aware of the apparent inconsistency between adding the names |
gold2718
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a question about units but this otherwise seems fine.
standard_names.xml
Outdated
| </standard_name> | ||
| <standard_name name="sea_water_salinity" | ||
| description="The salinity of sea water"> | ||
| <type units="ppt m">real</type> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am familiar with ppt for seawater salinity but not ppt m. What is this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know. (I'm a software engineer, not a scientist, so I'm probably not the best person to create this PR.) It could well be incorrect. I just copied the units in use for the existing sea_water_salinity_in_diurnal_thermocline variable, figuring they would have the same units. But, since I don't understand the diurnal_thermocline part, maybe that's incorrect. I'm happy to correct it to just ppt if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After a quick google search, it seems like the units for sea_water_salinity should either be ppt (parts per thousand), psu (practical salinity units, which seems to be the same numerical value as ppt), or possibly even just percent. Sorry for not looking into this matter more carefully originally. Can someone with ocean expertise advise on which units should be used here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
short answer: use PSU, and state "practical salinity of sea water" in the description
long answer:
Technically PSU is not a unit, so i've seen places where the unit is "1", but who cares.
Also, "salinity" is ambiguous, there are 2 main types of salinity 1) "practical salinity" which is what ocean models historically used to use, and is a measure of conductivity and 2) "absolute salinity" which is the more recent standard that models are now starting to use, and has units of g/kg
edit: and, fyi, ppt is close to psu, but technically not the same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should stick with PSU which is the current standard for measurements. It may be confusing because in Gibbs Seawater (GSW) toolbox, absolute salinity (AS) is used but there is a conversion from pracitcal salinity (PS) to AS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @travissluka. Since the goal of ESM naming is to avoid ambiguity, should the name of this variable be changed to sea_water_practical_salinity instead of sea_water_salinity? It is possibly only a matter of time until the more recent kind of salinity is added to the ESM naming, so we might as well prepare so we don't have to change names later. Or it is even simple enough to add both names now.
|
Tagging @twsearle |
| <standard_name name="sea_water_salinity" | ||
| description="The practical salinity of sea water"> | ||
| <type units="PSU">real</type> | ||
| </standard_name> | ||
| <standard_name name="sea_water_absolute_salinity" | ||
| description="The absolute salinity of sea water"> | ||
| <type units="g kg-1">real</type> | ||
| </standard_name> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took a look at the CF conventions, and added their names for these two quantities. It looks to me like CF uses ppt, not PSU for the units of sea_water_salinity. But I stuck with PSU here since that is what @Dooruk and @travissluka recommended.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I looked again, closer, and CF naming has three names:
sea_water_salinitywith units of1e-3(which I think means the same asppt?)sea_water_practical_salinitywith units of1(which Travis says is the same asPSU)sea_water_absolute_salinitywith units ofg kg-1
Do we want to duplicate all three names in ESM? I need scientific guidance here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Dooruk referred me to this issue.
Here are my suggestions and perspective on this issue:
sea_water_salinity with units of 1e-3 (which I believe is equivalent to ppt): I don’t think it is necessary to keep this variable, as it reflects an older and less precise definition of salinity.
sea_water_practical_salinity with units of 1 (which Travis noted is equivalent to PSU): This variable should be retained, as it is consistent with in-situ observations and is useful for validation and diagnostic analyses.
sea_water_absolute_salinity with units of g kg⁻¹: This represents the most recent and precise definition of salinity and is therefore also worth keeping.
If only one salinity variable were to be retained, I would lean toward sea_water_absolute_salinity, assuming it comes directly from the model output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @lren20 , for your insights. That's very helpful. Since these variables are all new to this ESM naming standard, we are free to do what makes sense and is clear for the modern community. We can look to CF naming for insight, but don't need to be constrained by it. Given what you've said, I propose we add two salinity names to ESM Naming in this PR:
- Either
sea_water_salinityorsea_water_practical_salinity, with units ofPSUeither way. Going withsea_water_salinityprobably means fewer changes in legacy code, but we would be changing the units for this name from the CF convention, so we're possibly introducing some confusion there. Conversely, usingsea_water_practical_salinityfor the ESM name conforms to the CF standard and is completely unambiguous, but probably requires more changes to legacy code, since thensea_water_salinitywill not be an ESM name. - Also add the name
sea_water_absolute_salinitywith units ofg kg-1, since this is the most recent and precise salinity definition.
Regarding the two options described in 1) above, currently this PR contains sea_water_salinity with units of PSU. I'm now leaning toward changing this name to sea_water_practical_salinity. Then the ESM names would be consistent with CF, but would be missing the older, somewhat ambiguous term sea_water_salinity.
I will make that change later today unless I hear objections.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I would rather stick with
sea_water_practical_salinitybut that would require SOCA changes and potential downstream impacts for EMC and GMAO. - Sounds good
i will ask @sinakhani and @guillaumevernieres input here (please see temperature unit comment above as well).
@ss421 I see this turned out to be a can of worms, we could also discuss it during the Thursday JCSDA meeting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather stick with sea_water_practical_salinity but that would require SOCA changes and potential downstream impacts for EMC and GMAO.
To be clear, it is only in JEDI generic code (such as VADER, SABER, and UFO) where we make a strong attempt to conform to using ESM names for the model variables. This is because it is most important in generic code to be clear and unambiguous about what the variables represent (and what units they have) since the code can be used by multiple models and organizations. So if, say, we choose to use the ESM name sea_water_practical_salinity in the VADER code, that doesn't mean all of SOCA needs to change all its code to using that name. It just means SOCA will need to do a variable name change when it passes the variable into VADER. This sort of thing is done in multiple model interfaces to JEDI.
We also need to make sure we know which salinity units are appropriate for the new VADER code that inspired this PR (referenced in the PR description), so that we are sure to have an appropriate name for use there. (i.e., if the code formulas in VADER are expecting salinity to be in ppt, then we better have a name with those units.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok thanks, that separation makes more sense now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think [ppt] is identical to [g/kg] based on their definitions. If I am also not wrong, 1 PSU should approximately equal to 1 ppt in oceanography (PSU is more recent definition for salinity but ppt is an older one). I expect all these three variables to be very close -- are they very different in your data outputs?
For temperature, I think it is a choice to use degC or K since they only differ by a constant 273.15. As long as the dataset gives a unit for temperature, either choice is fine in my view.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From a met office marine obs perspective, we use all three. But our models use only two 'sea_water_salinity' for EOS80 salinity, and 'sea_water_absolute_salinity' for TEOS-10 salinity.
| * `real`: units = m3 m-3 | ||
| ## marine | ||
| * `potential_temperature_of_sea_water`: sea water potential temperature | ||
| * `real`: units = K |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will comment here that while K is used in some products, degC is more common. MOM6 (model SOCA interfaces) outputs model fields in degC. Looking at a few common products I see they differ though, like so:
OISST, SODA, ORAS5 uses degC
EN4, OSTIA (GHRSST) uses K
I guess we can't make up our mind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a note here that GHRSST includes a lot of members from all over the world https://www.ghrsst.org/about-ghrsst/ not just OSTIA (Australian Bureau Of Meterology, NOAA, Canadian Meterological and Oceanographic Society, among many others from elsewhere in Europe etc). See here for a list:
https://www.ghrsst.org/ghrsst-data-services/for-sst-data-producers/ghrsst-catalogue/#/search?from=1&to=30
The GHRSST conventions follow the CF conventions as far as possible with a little extra metadata. Documented in GDS2.1 (page 83, convention is Kelvin for SST variable there).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Dooruk In the Standard Names dictionary the exact units are not binding, but rather guidelines on the dimensionality of the described quantity (https://github.com/ESCOMP/ESMStandardNames/blob/main/StandardNamesRules.rst#units). We prefer to use SI units wherever possible, since those are standard and unambiguous in their dimensionality, so K is preferred over any other temperature variable here.
| </standard_name> | ||
| </section> | ||
| <section name="marine"> | ||
| <standard_name name="potential_temperature_of_sea_water" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sea_water_potential_temperature to stay consistent with the salinity naming convention that you define below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sea_water_potential_temperatureto stay consistent with the salinity naming convention that you define below.
I've commented above on why I have this naming inconsistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@svahl991 Oops, sorry, I missed your comment. Should we follow the same convention for salinity then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we follow the same convention for salinity then?
Do you mean salinity_of_sea_water instead of sea_water_salinity? I thought about that, but decided it was silly since I don't think you would measure salinity of air. There's a whole list of rules about how to construct ESM names, and the first rule is to use CF names if a good one exists that doesn't cause naming inconsistencies. I feel the CF names for salinity are OK, and right now we're just deciding how many of the CF salinity names to add to ESM, and what units they should have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI @svahl991 : the CF name for sea water potential temperature is sea_water_potential_temperature ... Most of the naming convention in soca attemps to follow CF.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ESM (Earth System Model) standard names were created to provide consistency between Earth system models. While the original effort started with CF names only, it became clear very quickly that these wouldn't suffice. Hence, the next step was to add names where no CF names existed. It then became obvious that the CF names in some cases were inconsistent, misleading, or unclear, and that using the CF names would make it difficult to achieve the goals of the ESM standard names effort. At this point, it was decided that CF would be used, when possible, but that a deviation from the standard is acceptable otherwise.
Names in ESM standard names apply to all modeling systems using these names. That is, they are not model specific, and they provide compatibility and clarity across systems that use the ESM standard names, beyond what the sometimes ambiguous and extremely limited CF convention would be able to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@climbfuji agreed. I dont think reordering the potential temperature of sea water variable improves clarity or makes the name any different in terms of ambiguity from the cf name. I dont think consistency with atmosphere names is a good reason to deviate from the CF names if we don't need to personally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, the crux is that CF is inconsistent in that regard. You find names like air_temperature but also names like duration_of_sunshine instead of sunshine_duration, or divergence_of_wind instead of wind_divergence. Some names in CF have aliases, allowing both forms, and allow for even more inconsistency with prefices and suffices
atmosphere_mass_content_of_water_vapor
alias: atmosphere_water_vapor_content
One of the goals of ESMStandardNames is to provide a clear framework for how to construct new names. To do that, you need a clear set of rules for how to compose names. After reviewing the names in the CF conventions and the different, inconsistent ways to construct names based on those, we spent considerable effort to decide on one way to compose these names. Consequently, all names in the ESMStandardNames must follow these rules, even if that means that they differ from CF.
The rules are defined in https://github.com/ESCOMP/ESMStandardNames/blob/main/StandardNamesRules.rst. It basically comes down to "what is the base name" and what is a qualifier such as "component", "medium", ...
Looking at the rules and at this particular example, "sea_water_potential_temperature", I can see your point. For one, there is no prefix "potential_temperature" and I would argue that it hardly qualifies as a "component" of a base name. Also, just "sea_water" by itself is not really a base name? Is the base name then sea_water_potential_temperature? I tend to agree, but then I am questioning why the ESMStandardNames dictionary doesn't use air_potential_temperature but potential_temperature_of_air.
The longer you stare at this stuff, the more confusing it gets ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@climbfuji potential_temperature should be a base name, since it's a distinct quantity from general temperature This was an inadvertent omission; it looks like virtual_potential_temperature made it in but potential_temperature did not.
sea_water in this case is the medium as described in the rules here. But the documentation is very thin here and could probably be expanded/clarified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
potential_temperature should be a base name, since it's a distinct quantity from general temperature This was an inadvertent omission;
@mkavulich , So are you suggesting that maybe ESM should be changed to conform to the CF name for air_potential_temperature instead of potential_temperature_of_air. And then it would be consistent to add the CF name sea_water_potential_temperature? I would support this decision.
Description
We need some marine variables added to the ESM standard since we wish to use them in generic JEDI code for use with SOCA. I have attempted to follow the conventions used for the corresponding
airvariables.Issues
Variables needed for https://github.com/JCSDA-internal/vader/pull/208